feat(setup): inject Windows-node guidance into AGENTS.md after pairing - #659
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: the refreshed branch removed the permissive Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. So I’m closing this here because the remaining work is already tracked in the canonical issue. Review detailsBest possible solution: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. Do we have a high-confidence way to reproduce the issue? Not applicable as a user bug report, but the PR blockers are source-reproducible: the managed payload is broad at Is this the best way to solve the issue? No. The safer solution is scoped, explicit Windows-node guidance plus best-effort setup enrichment, with current-head proof of the revised setup behavior. Security review: Security review needs attention: The permissive exec-default advice was removed, but workspace-global routing guidance still has a concrete security-boundary concern for remote or multi-node sessions.
AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against c9b5476a873f. |
clawsweeper
Bot
added
proof: sufficient
|
just as a heads up -- this change is a little wonky in terms of what we're doing... it works, but I wouldn't say that it's elegant. anyways, here's a completely clean install being asked to pop a notification: |
|
@paulcam206 does it only do it if the message is coming in via the Windows app? |
|
Automated Copilot review note from the wide PR triage pass: I would not treat this as safe to merge at 90% confidence yet, even aside from the current conflict state. The core concern is scope: this PR injects Windows-node guidance into the gateway workspace's global AGENTS.md, so it can affect agent behavior for requests that did not originate from the local Windows tray chat — for example Telegram-originated work, remote/gateway sessions, or future multi-node scenarios. Two things I think should be tightened before merge:
Separately, because this is post-pairing enrichment, I would prefer injection failures/timeouts/malformed existing markers to be non-terminal or self-healing where possible. Optional guidance should not block onboarding after pairing succeeds. |
|
Thanks for the proof and the implementation here. I don't think this is safe to merge as-is yet because the generated Could you please revise this so the injected guidance is clearly scoped to tasks that intentionally need this paired Windows node, remove or soften the permissive |
clawsweeper
Bot
added
proof: 📸 screenshot
Add WindowsNodeBootstrapContextStep that runs after node pairing and injects a managed BEGIN/END block into the gateway's AGENTS.md (workspace resolved from `openclaw config get agents.defaults.workspace`, or from a `WindowsNodeContext.WorkspacePath` override). The injected block tells the running OpenClaw agent how to use the paired Windows tray node (nodes tool, exec host=node, tools.exec defaults, etc.). The file mutation is pure POSIX shell + awk + base64 — no Node.js dependency, no embedded JS, no heredocs. Bare WSL distros (and the gateway distro) do not have node installed, so the previous node-based approach failed at runtime. Scripts that need bash variable handling are piped to `bash -s` via stdin instead of being passed as argv to `bash -c`, because wsl.exe performs shell variable expansion on argv before bash sees it and silently drops user-defined $var references. New docs/WSL_EXE_ARGV_PITFALL.md documents the footgun, the empirical reproduction against fresh Ubuntu-26.04, and the ranked fixes. Notable changes: * CommandRunner.RunInWslAsync gains opt-in `inputViaStdin: true` that switches argv to `bash -s` and pipes the script over stdin * WindowsNodeBootstrapContextStep computes the absolute workspace path once via ExpandLinuxPath and threads it through both `openclaw setup --workspace` and the apply script, so `~/foo` or relative-path overrides cannot land in different directories * RunOpenclawSetupAsync and ResolveWorkspacePathAsync use the stdin path because their scripts include $PATH via WslPathPrefix * Apply script is idempotent and handles missing/symlink/malformed AGENTS.md; rollback removes the managed block in-place * Cross-references in AGENTS.md and docs/WINDOWS_NODE_TESTING.md surface the pitfall doc from likely discovery points * Tests cover apply/rollback shape, override expansion, per-call stdin-vs-argv invariants, and a tilde-override regression * Verified end-to-end against bare Ubuntu-26.04 (no openclaw, no node): apply-to-missing, apply-preserves-content, idempotency (exactly 1 BEGIN marker after second apply), rollback restores original content Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
removed
rating: 🧂 unranked krab
clawsweeper
Bot
removed
proof: 📸 screenshot
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69b85fbfa8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (result.TimedOut || result.ExitCode != 0) | ||
| return null; |
There was a problem hiding this comment.
In the explicit skip-onboard path (SkipWizardAsync or RunGatewayWizardStep.CanSkip), no agent workspace may have been initialized yet because the baseline initializer runs later in this step. Returning null as soon as openclaw agents list --json exits non-zero makes Windows-node context finalization fail before consulting agents.defaults.workspace or running openclaw setup --workspace, so a fresh local setup where the user chooses to skip OpenClaw onboard can get stuck instead of completing with the default workspace. Please fall back to the configured/default workspace when the agents list is unavailable.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: 📸 screenshot
3 participants
Summary
Adds a setup-owned Windows-node context block to the effective main/default agent workspace after onboarding. This teaches the gateway agent to route Windows work through the paired tray node instead of attempting Windows-only operations inside WSL.
The maintainer pass integrates the feature into the native onboarding wizard and hardens it for upgrades, retries, custom agent workspaces, and deterministic uninstall.
Behavior
openclaw agents list --json; per-agent overrides do not rewrite global defaults.openclaw setup --baselinewhile retaining compatibility with the pinned 2026.6.11 gateway CLI.AGENTS.md, preserving unrelated content, CRLF, and existing file mode.wsl.execannot mutate shell variables in the script argument.Implementation notes
The file mutation remains POSIX-shell based and does not assume Node.js exists in a fresh WSL distro. Managed markers make repeated application idempotent and rollback scoped. A focused WSL argv reference documents why scripts containing shell variables must be sent through stdin.
Validation
OpenClaw.SetupEngine.Tests418/418 passed.HCS_E_HYPERV_NOT_INSTALLEDwith actionable recovery text at the expected boundary.Co-authored with and based on the original contribution from @paulcam206.